Skip to content

Conversation

@kai-ion
Copy link
Contributor

@kai-ion kai-ion commented Aug 1, 2025

Issue #, if available:

Description of changes:

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed new line in file without changing file

{
// Add S3Express user agent feature if backend is S3Express
if (endpoint.GetAttributes() && endpoint.GetAttributes()->backend == "S3Express") {
const_cast<Aws::AmazonWebServiceRequest&>(request).AddUserAgentFeature(Aws::Client::UserAgentFeature::S3_EXPRESS_BUCKET);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we adding the cast const_cast<Aws::AmazonWebServiceRequest&> ? its not needed?

EXPECT_FALSE(response.IsSuccess());

// Verify S3_EXPRESS_BUCKET feature (value 6) was added
const auto& features = request.GetUserAgentFeatures();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isnt actually testing it the other user agent test captures the request that was seen in the http client -- this is not using the captured mock request, it is jsut using the request that you submitted to the s3 client

@kai-ion kai-ion force-pushed the mato branch 2 times, most recently from 581d37f to f5a8524 Compare August 6, 2025 20:06
- added S3_express_bucket tracking

updated to only include tracking for s3_express

updated to only include tracking for s3_express

updated to only include tracking for s3_express

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

s3 unit test

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent

updated to only include tracking for s3_express
one level deeper to AWSXmlClient

Updated S3 unit test for correct user agent
@kai-ion kai-ion merged commit 995bc6e into main Aug 7, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants